@Sanne, you should be able to test Quarkus with this branch without any additional changes but I'm preparing a branch Quarkus that uses the new API

you should be able to test Quarkus with this branch without any additional changes ...

Tested it, that worked fine.

but I'm preparing a branch Quarkus that uses the new API

Nice, let me know when you have something.

Look, I'm not keen on the idea that we're changing the name of a public API because of some transient integration issue.

If we really think newSession() is a better name, then OK, fine. To me it doesn't seem better.

@gavinking we're open to suggestions of course.

I did suggest newSession() because the other method one could use - withSession - does leverage the vertx context, which confused me initially as it might reuse some existing session in the context (which honestly surprised me).

So I think newSession nicely expresses the aspect that this one method actually guarantees it to be a new one. And this conveys a suggestion about when this is a good choice, as otherwise I'd normally always prefer to use withSession which IMO looks better when working with reactive.

Would you like to suggest some alternatives?

Well I was very happy with openSession() which to me also pretty unambiguously signals a new session, and is the naming that's used in Hibernate ORM.

createSession also works for me.

Or openSessionUni and openSessionStage

Well I was very happy with openSession() which to me also pretty unambiguously signals a new session, and is the naming that's used in Hibernate ORM.

while it's not legal in source code we can have both flavours of openSession() in the API; it needs a little tooling gymnastics to make it happen though, so I'd only do that if you feel it's important.

What I'm saying is I don't find your justification for not just removing the previous versions to be very convincing.

What I'm saying is I don't find your justification for not just removing the previous versions to be very convincing.

(It puts the cart before the horse.)

What I'm saying is I don't find your justification for not just removing the previous versions to be very convincing.

oh yes sure, we initially tried to remove it.

On the Quarkus side we're having two Panache tests failing for reasons we couldn't understand yet; we spent some days on this and that's why this is coming a week later than when we last discussed it - I then suggested to Davide that if it was possible to release another CR which had both methods, we could then upgrade Quarkus and migrate one module at a time - and bisect the problem, which is eluding us currently.

So another alternative is we try harded to figure it out on the Quarkus side :) if we solve that we could just delete them, but then also deprecating these would feel more like doing the right thing for the few users we already have.

Why don't you just add some method that Panache needs to the new Implementor interface and do a typecast in the Quarkus code?

Why don't you just add some method that Panache needs to the new Implementor interface and do a typecast in the Quarkus code?

brilliant :)

Thanks

@DavideD let's do that? It's not as nice to other users but will definitely help us move on.

It's not as nice to other users

I mean, to me it seems much better than releasing 1.0 with some operations on a public API that you're already planning on removing later.

I mean, to me tat seems much better than releasing 1.0 with some operations on a public API that you're already planning on removing later.

well to be clear I was going to make this another CR, and then remove it before tagging Final.

But sure let's use the Implementor instead.

I've updated this PR and moved the mehods required for Quarkus in the interface MutinyImplementor.
I've also created StageImplementor but decided to remove it because we don't need it in Quarkus.

@Sanne to make this work with Quarkus you need this commit: DavideD/quarkus@3807d77

The rest of the branch contains all the other changes we applied to Quarkus before that make everything work except for Panache